Skip to content

Conversation

@petkostas
Copy link
Contributor

@petkostas petkostas commented Oct 1, 2025

Summary

  • Bump and align GO to version 1.23
  • Update failing linting issues
  • Pin golang-ci version

Does this close any open issues?

Closes #8584

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. component/framework This issue or PR relates to the framework improvement priority/medium This issue is medium important labels Oct 1, 2025
@spenpal
Copy link

spenpal commented Oct 3, 2025

This PR looks good to me. If possible, can you resolve the version issue with the go-dev-tools make command requiring Go 1.24?

@spenpal
Copy link

spenpal commented Oct 5, 2025

Another issue: When running make dep, I am get the following error:

go: github.com/golangci/golangci-lint/cmd/golangci-lint@latest: github.com/golangci/[email protected] requires go >= 1.23.0 (running go 1.22.12; GOTOOLCHAIN=local)

Do we need a higher version than Go 1.22? The last golangci-lint that supports Go 1.22 is v1.63.4, FYI. Currently, it is being set to latest in the backend Makefile.

@petkostas petkostas force-pushed the chore/8584-update-go branch from c0a09c8 to 258e964 Compare October 9, 2025 15:40
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Oct 9, 2025
@petkostas petkostas force-pushed the chore/8584-update-go branch from b026f34 to 281fe9f Compare October 9, 2025 16:25
@petkostas
Copy link
Contributor Author

@spenpal going to 1.23 was a bit more tricky, but looks good now.

@petkostas petkostas changed the title chore(go): update go and align versions chore(go): update go version Oct 9, 2025
klesh
klesh previously approved these changes Oct 11, 2025
Copy link
Contributor

@klesh klesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Feel free to merge it when ready.

@spenpal
Copy link

spenpal commented Oct 14, 2025

Sorry for the late reply.

@klesh Is make go-dev-tools required for contributing to Apache Devlake? Because it requires Go 1.24.

make go-dev-tools
# go install github.com/atombender/go-jsonschema/cmd/gojsonschema@latest
go install golang.org/x/tools/cmd/goimports@latest
go: downloading golang.org/x/tools v0.38.0
go: golang.org/x/[email protected] requires go >= 1.24.0; switching to go1.24.9
go: downloading go1.24.9 (darwin/arm64)
go: downloading golang.org/x/telemetry v0.0.0-20251008203120-078029d740a8
go: downloading golang.org/x/mod v0.29.0
go install golang.org/x/tools/gopls@latest
go: golang.org/x/tools/[email protected] requires go >= 1.24.2; switching to go1.24.9
go install github.com/go-delve/delve/cmd/dlv@latest
go install honnef.co/go/tools/cmd/staticcheck@latest
go: honnef.co/go/[email protected] requires go >= 1.23; switching to go1.24.9

If it is, @petkostas we might need to update to Go 1.24 or downgrade go-dev-tools to use versions that support Go 1.23.

@petkostas petkostas force-pushed the chore/8584-update-go branch 2 times, most recently from 2295488 to 3dd2fa4 Compare October 15, 2025 15:42
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 15, 2025
- Bump golangci-lint for GO v1.24
@petkostas petkostas force-pushed the chore/8584-update-go branch from 3dd2fa4 to a9ec769 Compare October 15, 2025 15:50
@petkostas
Copy link
Contributor Author

@spenpal Updated to v1.24 please give it a GO (😄) again.

@spenpal
Copy link

spenpal commented Oct 21, 2025

@petkostas This PR looks good to me now. Thanks for your hard work!

EDIT: I lied. I am unable to run make lint in the backend directory. Seems like upgrading to Go 1.24 has different config requirements for the .golangci.yaml file.

EDIT 2: I asked AI to upgrade the .golangci.yaml file for Go 1.24, and ran the make lint command. It said it found over 10k+ code quality issues...😅. Let me know if you need any help with this, @petkostas.

Here is the upgraded .golangci.yaml file, as reference (not sure if it is completely correct or accurate for this project, you can help verify).

# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements.  See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License.  You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

version: "2"

# https://golangci-lint.run/usage/linters/
linters:
  # Start with standard linters (enabled by default)
  default: standard
  
  enable:
    # Additional recommended linters for production code
    - bodyclose        # checks whether HTTP response body is closed
    - copyloopvar      # detects loop variable issues (Go 1.22+)
    - errcheck         # checks for unchecked errors
    - errorlint        # finds code that causes problems with Go 1.13+ error wrapping
    - gocritic         # provides diagnostics that check for bugs, performance and style
    - govet            # reports suspicious constructs
    - ineffassign      # detects unused assignments
    - makezero         # finds slice declarations with non-zero initial length
    - misspell         # finds commonly misspelled English words
    - nolintlint       # reports ill-formed or insufficient nolint directives
    - revive           # fast, configurable, extensible, flexible linter
    - staticcheck      # set of rules from staticcheck (includes gosimple, stylecheck)
    - unconvert        # removes unnecessary type conversions
    - unparam          # reports unused function parameters
    - unused           # checks for unused constants, variables, functions and types
    - goheader         # checks file headers match template
    - importas         # enforces consistent import aliases

  disable:
    # Disable overly strict or noisy linters
    - exhaustive       # too strict for most codebases
    - funlen           # arbitrary function length limits
    - gochecknoglobals # globals are sometimes necessary
    - gocognit         # cognitive complexity can be subjective
    - gocyclo          # cyclomatic complexity can be subjective
    - godox            # don't want to fail on TODO/FIXME comments
    - mnd              # magic number detection too strict (was gomnd)
    - lll              # line length better handled by formatters
    - nlreturn         # overly opinionated about blank lines
    - testpackage      # not always beneficial to use _test package
    - wsl              # overly opinionated whitespace rules

formatters:
  enable:
    - gofmt      # Format code with gofmt
    - goimports  # Organize imports

linters-settings:
  goheader:
    template-path: .golangci-goheader.template

  goimports:
    # Put local imports after 3rd-party packages
    local-prefixes: github.com/apache/incubator-devlake

  staticcheck:
    # Enable all checks
    checks: ["all"]

  revive:
    # Maximum number of open files at the same time
    max-open-files: 2048
    # Sets the default severity
    severity: error
    
    rules:
      # Enable useful rules
      - name: atomic
      - name: blank-imports
      - name: bool-literal-in-expr
      - name: call-to-gc
      - name: confusing-naming
      - name: confusing-results
      - name: constant-logical-expr
      - name: context-as-argument
      - name: context-keys-type
      - name: deep-exit
      - name: defer
        arguments: [["call-chain"]]
      - name: dot-imports
      - name: duplicated-imports
      - name: empty-block
      - name: empty-lines
      - name: error-naming
      - name: error-return
      - name: error-strings
      - name: errorf
      - name: get-return
      - name: identical-branches
      - name: import-shadowing
      - name: modifies-parameter
      - name: modifies-value-receiver
      - name: optimize-operands-order
      - name: package-comments
      - name: range
      - name: range-val-in-closure
      - name: range-val-address
      - name: receiver-naming
      - name: redefines-builtin-id
      - name: string-of-int
      - name: string-format
        arguments:
          - - "core.WriteError[1].Message"
            - "/^([^A-Z]|$)/"
            - must not start with a capital letter
          - - "fmt.Errorf[0]"
            - '/(^|[^\.!?])$/'
            - must not end in punctuation
          - - panic
            - '/^[^\n]*$/'
            - must not contain line breaks
      - name: struct-tag
      - name: superfluous-else
      - name: time-equal
      - name: time-naming
      - name: var-declaration
      - name: unconditional-recursion
      - name: unexported-return
      - name: unhandled-error
        arguments:
          - "fmt.Printf"
          - "fmt.Println"
          - "fmt.Fprintln"
          - "res.Body.Close"
          - "body.Close"
      - name: unnecessary-stmt
      - name: unreachable-code
      - name: useless-break
      - name: waitgroup-by-value
      
      # Disable overly strict rules
      - name: bare-return
        disabled: true
      - name: exported
        disabled: true
        arguments:
          - "disableStutteringCheck"
      - name: var-naming
        disabled: true
        arguments:
          - [] # AllowList
          - [] # DenyList

  gocritic:
    # Enable multiple check groups
    enabled-tags:
      - diagnostic
      - style
      - performance
      - opinionated
    disabled-checks:
      - whyNoLint # we use nolintlint for this
      - unnamedResult # conflicts with some patterns

  errcheck:
    # Report about not checking of errors in type assertions: `a := b.(MyStruct)`
    check-type-assertions: true
    # Report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`
    check-blank: false
    # List of functions to exclude from checking
    exclude-functions:
      - (io.Closer).Close
      - (*database/sql.Rows).Close

  errorlint:
    # Check whether fmt.Errorf uses the %w verb for formatting errors
    errorf: true
    # Check for plain error comparisons
    comparison: true

  nolintlint:
    # Enable to require an explanation of nonzero length after each nolint directive
    require-explanation: true
    # Enable to require nolint directives to mention the specific linter being suppressed
    require-specific: true

  importas:
    # Enforce import aliases for consistency
    no-unaliased: false
    # Do not allow non-required aliases
    no-extra-aliases: false

issues:
  # Maximum issues count per one linter. Set to 0 to disable.
  max-issues-per-linter: 0
  # Maximum count of issues with the same text. Set to 0 to disable.
  max-same-issues: 0

  # Include issues that were excluded by default exclusion patterns
  include:
    - EXC0012 # revive: comment on exported items
    - EXC0014 # revive: comment on exported items

  exclude-rules:
    # Exclude some linters from running on specific paths
    - path: models/|api/|migration/|errors/|logger/
      linters:
        - revive
    - path: plugins/([^hc].*|.[^eo].*|..[^lr].*|...[^pe].*|....[^e].*|.....[^r].*)
      linters:
        - revive
    # Exclude test files from some checks
    - path: _test\.go
      linters:
        - errcheck
        - gocritic
        - revive
    # Exclude generated files
    - path: ".*\\.pb\\.go$"
      linters:
        - all
    - path: ".*\\.gen\\.go$"
      linters:
        - all

# Options for analysis running
run:
  # The default concurrency value is the number of available CPUs
  concurrency: 4
  # Timeout for analysis
  timeout: 5m
  # Exit code when at least one issue was found
  issues-exit-code: 2
  # Include test files or not
  tests: false
  # List of build tags
  build-tags: []
  # Allow multiple parallel golangci-lint instances running
  allow-parallel-runners: true
  # Allow multiple golangci-lint instances running, but serialize writes to any files
  allow-serial-runners: false
  # Use go modules for dependency resolution
  modules-download-mode: readonly

# Output configuration
output:
  # Format: colored-line-number, line-number, json, tab, checkstyle, code-climate, html, junit-xml, github-actions
  formats:
    colored-line-number:
      path: stdout
  # Print lines of code with issue
  print-issued-lines: true
  # Print linter name in the end of issue text
  print-linter-name: true
  # Make issues output unique by line
  uniq-by-line: true
  # Sort results by: filepath, line and column
  sort-results: true
  # Show statistics per linter
  show-stats: true
> make lint
...
...

10711 issues:
* bodyclose: 74
* copyloopvar: 1
* errcheck: 252
* errorlint: 21
* gocritic: 205
* goimports: 353
* govet: 2
* misspell: 13
* nolintlint: 33
* revive: 9623
* staticcheck: 96
* unconvert: 4
* unparam: 34

@petkostas
Copy link
Contributor Author

Thanks @spenpal I will try to look into that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/framework This issue or PR relates to the framework improvement priority/medium This issue is medium important size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][Backend] Go version inconsistency between go.mod (1.20) and development environment (1.22)

3 participants